refactor(wait_for_status): Use self.instance to get status#2499
Conversation
WalkthroughReworks wait_for_status in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 💡 Knowledge Base configuration:
You can enable these settings in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
ocp_resources/resource.py (1)
966-974: Respect the Resource.status accessor and tolerate transient NotFound/AttributeErrorPolling self.instance.status.phase bypasses per-resource status logic. I scanned the repo and found resource.status is a @Property at ocp_resources/resource.py:1052 and node_network_configuration_policy.status is a @Property at ocp_resources/node_network_configuration_policy.py:364 — so use the resource accessor instead of touching self.instance directly. Also the TimeoutSampler at ocp_resources/resource.py:966–974 omits NOT_FOUND handling (other samplers e.g. around line 905 include it) and should tolerate AttributeError while status fields settle.
Please apply this change:
- Locations to fix:
- ocp_resources/resource.py — TimeoutSampler at ~lines 966–974 (replace polling and exceptions)
- Review other TimeoutSampler usages for consistency: lines 901, 927, 966, 1109, 1234, 1242, 1294, 1302
Suggested diff:
samples = TimeoutSampler( wait_timeout=timeout, sleep=sleep, - exceptions_dict={ - **PROTOCOL_ERROR_EXCEPTION_DICT, - **DEFAULT_CLUSTER_RETRY_EXCEPTIONS, - }, - func=lambda: self.instance.status.phase, + exceptions_dict={ + **PROTOCOL_ERROR_EXCEPTION_DICT, + **DEFAULT_CLUSTER_RETRY_EXCEPTIONS, + **NOT_FOUND_ERROR_EXCEPTION_DICT, + AttributeError: [], + }, + # Respect per-resource status implementations (support method or property). + func=lambda: (self.status() if callable(getattr(self, "status", None)) else getattr(self, "status", None)), )This preserves per-resource overrides and avoids premature failures while a resource's status settles.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ocp_resources/resource.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
ocp_resources/resource.py (1)
ocp_resources/node_network_configuration_policy.py (1)
status(364-367)
…nto wait_for_status-use-instance
…nto wait_for_status-use-instance
|
/verified |
8 participants
Summary by CodeRabbit
New Features
Improvements
Bug Fixes